Skip to content

Pet controller success msg#2362

Open
WJunn987 wants to merge 28 commits into
spring-projects:mainfrom
ky7721:PetControllerSuccessMsg
Open

Pet controller success msg#2362
WJunn987 wants to merge 28 commits into
spring-projects:mainfrom
ky7721:PetControllerSuccessMsg

Conversation

@WJunn987
Copy link
Copy Markdown

No description provided.

ky7721 and others added 27 commits April 17, 2026 16:02
Update UI Text labels in vet list page
Update the Successfull messages for PetControllerr
Signed-off-by: WJunn987 <ongweijun@1utar.my>
Update Pet Controller Add Successfull Msg
Signed-off-by: WJunn987 <ongweijun@1utar.my>
Updated Pet Controller Successfull Msg
Copilot AI review requested due to automatic review settings April 19, 2026 06:01
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates some user-facing UI strings (pet success flash messages and vet list labels), but also includes a breaking rewrite of the vet persistence layer file.

Changes:

  • Update PetController flash success messages for pet create/update.
  • Adjust fallback (non-i18n) text in the vets list template.
  • Replace VetRepository.java contents with controller code (introducing compilation issues and duplicate controller definitions).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
src/main/resources/templates/vets/vetList.html Changes fallback text for vet list heading/columns (rendered text still comes from i18n keys).
src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java Replaces repository interface with a controller class (breaks compilation, duplicates existing VetController).
src/main/java/org/springframework/samples/petclinic/owner/PetController.java Updates pet creation/update flash success messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

<body>

<h2 th:text="#{vets}">Veterinarians</h2>
<h2 th:text="#{vets}">Veterinarian Directory</h2>
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Thymeleaf, th:text="#\{vets\}" replaces the element body with the value from the message bundle (messages*.properties). Since vets is defined (e.g., messages.properties:20), the new fallback text "Veterinarian Directory" will never be shown in normal operation. If the intent is to change the visible heading, update the vets message value (and translations) or use a new message key.

Suggested change
<h2 th:text="#{vets}">Veterinarian Directory</h2>
<h2 th:text="#{vets}">Vets</h2>

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +13
<th th:text="#{name}">Veterinarian Name</th>
<th th:text="#{specialties}">Areas of Expertise</th>
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These header text changes are only fallbacks; the actual rendered text comes from #{name} and #{specialties} (both defined in messages*.properties), so the page will still display the translated "Name" / "Specialties" strings. If you want the UI to read "Veterinarian Name" / "Areas of Expertise", introduce dedicated message keys (and translations) or change the existing message values (not just the fallback text).

Suggested change
<th th:text="#{name}">Veterinarian Name</th>
<th th:text="#{specialties}">Areas of Expertise</th>
<th th:text="'Veterinarian Name'">Veterinarian Name</th>
<th th:text="'Areas of Expertise'">Areas of Expertise</th>

Copilot uses AI. Check for mistakes.
owner.addPet(pet);
this.owners.save(owner);
redirectAttributes.addFlashAttribute("message", "New Pet has been Added");
redirectAttributes.addFlashAttribute("message", "Pet added successfully.");
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These success messages now use sentence case with trailing periods, but other controllers use title case without punctuation (e.g., OwnerController: "New Owner Created", VisitController: "Your visit has been booked"). Consider aligning the message style across controllers for consistent UI, or move these to i18n message bundles if you want more control over copy/translation.

Suggested change
redirectAttributes.addFlashAttribute("message", "Pet added successfully.");
redirectAttributes.addFlashAttribute("message", "Pet Added Successfully");

Copilot uses AI. Check for mistakes.

updatePetDetails(owner, pet);
redirectAttributes.addFlashAttribute("message", "Pet details has been edited");
redirectAttributes.addFlashAttribute("message", "Pet details updated successfully.");
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These success messages now use sentence case with trailing periods, but other controllers use title case without punctuation (e.g., OwnerController: "Owner Values Updated"). Consider aligning the message style across controllers for consistent UI, or move these to i18n message bundles if you want more control over copy/translation.

Suggested change
redirectAttributes.addFlashAttribute("message", "Pet details updated successfully.");
redirectAttributes.addFlashAttribute("message", "Pet Details Updated");

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +24
@Controller
class VetController {

private static final int PAGE_SIZE = 5;

Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title suggests this is only about PetController success messaging, but this change set also rewrites vet UI text and replaces VetRepository with controller logic. If these vet changes are unintended, consider splitting them into a separate PR (or dropping them) to keep scope focused and reviewable.

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +28
@Controller
class VetController {

private static final int PAGE_SIZE = 5;

private final VetRepository vetRepository;

public VetController(VetRepository vetRepository) {
this.vetRepository = vetRepository;
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VetRepository.java no longer defines a VetRepository interface; it now contains a @Controller VetController class and references a VetRepository type that doesn't exist anywhere in the codebase. This will break compilation and also duplicates org.springframework.samples.petclinic.vet.VetController which already exists in VetController.java. Restore VetRepository as a Spring Data repository interface, and keep/move controller logic in VetController.java (or rename/move the controller to a correctly named file).

Copilot uses AI. Check for mistakes.
Comment on lines 1 to 2
package org.springframework.samples.petclinic.vet;

Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Apache 2.0 license header that is present in other source files (e.g., VetController.java, PetController.java) has been removed from this file. Please restore the standard header to keep licensing consistent across the codebase.

Copilot uses AI. Check for mistakes.
Signed-off-by: WJunn987 <ongweijun@1utar.my>
RedirectAttributes redirectAttributes) {
if (result.hasErrors()) {
redirectAttributes.addFlashAttribute("error", "There was an error in updating the owner.");
redirectAttributes.addFlashAttribute("error", "Error updating the owner.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it's better the the old one?

</tr>
<tr>
<th th:text="#{address}">Address</th>
<th th:text="#{address}">Owner's Address</th>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're inside Owner table, and there is a label

Pet Owners Directory

not need to be Owner prefix in the columns.

</tr>
<tr>
<th th:text="#{city}">City</th>
<th th:text="#{city}">Owner's City</th>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're inside Owner table, and there is a label

Pet Owners Directory

not need to be Owner prefix in the columns.

</tr>
<tr>
<th th:text="#{telephone}">Telephone</th>
<th th:text="#{telephone}">Owner's Telephone</th>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're inside Owner table, and there is a label

Pet Owners Directory

not need to be Owner prefix in the columns.

<tr>
<th th:text="#{name}" style="width: 150px;">Name</th>
<th th:text="#{address}" style="width: 200px;">Address</th>
<th th:text="#{name}" style="width: 150px;">Owner Name</th>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're inside Owner table, and there is a label

Pet Owners Directory

not need to be Owner prefix in the columns.

Copy link
Copy Markdown
Contributor

@andrzejsydor andrzejsydor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants